Recompute fuse XY configs when overwriting#10
Open
msayr wants to merge 4 commits into
Open
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84ab6be117
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fused_configs = get_fused_configs(config_path, | ||
| 0.1) | ||
| 0.1, | ||
| overwrite=overwrite) |
There was a problem hiding this comment.
When --overwrite is used after a prior fusion, this now removes the cached configs and recomputes them before any old fused output stores are deleted. The recomputation path (create_configs_fused_stacks via get_ordered_datasets) globs every directory under output_path/xy_intermediate except masks/flow/10x, while fuse_stacks_group writes prior outputs back into that same directory as *_fused, so reruns can treat already-fused datasets as input stacks and generate recursive/incorrect fuse groups. Exclude existing fused outputs or remove them before regenerating configs.
Useful? React with 👍 / 👎.
### Motivation - `fuse_stacks_xy` could reuse existing `fuse_xy*.json` config files instead of recomputing them from the current `xy_intermediate` outputs, producing stale fuse groups and causing datasets (e.g. `g0000_t0000`) to be omitted from fusion. - Provide an explicit way to force recomputation when a user requests `--overwrite` so fuse configs reflect the current aligned stacks. ### Description - Add an `overwrite` parameter to `get_fused_configs` and propagate `overwrite` from `align_fused_stacks_xy` to `get_fused_configs` in `emalign/scripts/fuse_stacks_xy.py`. - When `overwrite` is true, delete any existing `fuse_xy*.json` files before regenerating them so configs are rebuilt from the current `xy_intermediate` datasets. - Emit a warning when loading pre-existing `fuse_xy*.json` files to alert users that cached fuse configs can be stale and that they can rerun with `--overwrite` to recompute. - No changes to the fuse algorithm itself; this is a correctness/regen behavior change to ensure configs match current data. ### Testing - Compiled the modified file with `python -m py_compile emalign/scripts/fuse_stacks_xy.py` and the compilation succeeded.
…improve config handling ### Motivation - Provide a way to regenerate derived `fuse_xy*.json` configuration files when `xy_intermediate` stacks change so stale configs don't get reused. - Allow the script to be executed directly from a source checkout (e.g. `python emalign/scripts/fuse_stacks_xy.py`). - Make config loading behavior clearer by logging when cached configs are used and by offering a forced recompute option. ### Description - Add a small `sys.path` insertion so the script can be run directly from the repository without requiring package installation. - Extend `get_fused_configs` with a `recompute` bool parameter and implement removal of existing `fuse_xy*.json` files when `recompute=True`. - Add a warning log when existing `fuse_xy*.json` files are loaded to indicate they may be stale. - Thread the `recompute_configs` flag through `align_fused_stacks_xy` and expose it on the CLI as `--recompute-configs` so users can delete and rebuild cached configs before running. ### Testing - No automated tests were run for this change.
…support direct script execution ### Motivation - Prevent stale `fuse_xy*.json` configurations from being reused when underlying `xy_intermediate` datasets have changed. - Avoid crashes or incorrect behavior when datasets are incomplete by skipping dataset folders that lack `.zattrs`. - Make the `fuse_stacks_xy.py` script runnable directly from a source checkout for easier local invocation and debugging. ### Description - Added a `--recompute-configs` CLI flag and `recompute`/`recompute_configs` parameters to `get_fused_configs` and `align_fused_stacks_xy` to allow deleting and regenerating `fuse_xy*.json` files before processing. - Implemented logic in `get_fused_configs` to remove existing `fuse_xy*.json` files when `recompute=True` and to warn when existing configs are loaded to remind users to use the recompute flag. - Updated `get_ordered_datasets` calls to exclude already fused datasets and added a guard to skip dataset directories that are missing `.zattrs` with a `logging.warning`. - Made `fuse_stacks_xy.py` import-safe when executed directly from a source tree by inserting the package root into `sys.path` when `__package__` is unset. ### Testing - Performed module import and CLI smoke checks by invoking the script help with `python -m emalign.scripts.fuse_stacks_xy -h` and `python emalign/scripts/fuse_stacks_xy.py -h`, which completed successfully. - Ran a quick import of the modified modules to verify there are no syntax errors, which succeeded. - No automated unit tests were added or modified as part of this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
fuse_stacks_xycould reuse existingfuse_xy*.jsonconfig files instead of recomputing them from the currentxy_intermediateoutputs, producing stale fuse groups and causing datasets (e.g.g0000_t0000) to be omitted from fusion.--overwriteso fuse configs reflect the current aligned stacks.Description
overwriteparameter toget_fused_configsand propagateoverwritefromalign_fused_stacks_xytoget_fused_configsinemalign/scripts/fuse_stacks_xy.py.overwriteis true, delete any existingfuse_xy*.jsonfiles before regenerating them so configs are rebuilt from the currentxy_intermediatedatasets.fuse_xy*.jsonfiles to alert users that cached fuse configs can be stale and that they can rerun with--overwriteto recompute.Testing
python -m py_compile emalign/scripts/fuse_stacks_xy.pyand the compilation succeeded.Codex Task